Skip to content

Conversation

vbvictor
Copy link
Contributor

No description provided.

@llvmbot
Copy link
Member

llvmbot commented Sep 24, 2025

@llvm/pr-subscribers-github-workflow

Author: Baranov Victor (vbvictor)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/160551.diff

1 Files Affected:

  • (modified) .github/workflows/pr-code-lint.yml (+11-3)
diff --git a/.github/workflows/pr-code-lint.yml b/.github/workflows/pr-code-lint.yml
index bc70933147bd2..caa2a134c521d 100644
--- a/.github/workflows/pr-code-lint.yml
+++ b/.github/workflows/pr-code-lint.yml
@@ -19,8 +19,6 @@ jobs:
     defaults:
       run:
         shell: bash
-    container:
-      image: 'ghcr.io/llvm/ci-ubuntu-24.04:latest'
     timeout-minutes: 60
     concurrency:
       group: ${{ github.workflow }}-${{ github.ref }}
@@ -47,10 +45,13 @@ jobs:
           echo "Changed files:"
           echo "$CHANGED_FILES"
 
+      # The clang tidy version should always be upgraded to the first version
+      # of a release cycle (x.1.0) or the last version of a release cycle, or
+      # if there have been relevant clang-format backports.
       - name: Install clang-tidy
         uses: aminya/setup-cpp@17c11551771948abc5752bbf3183482567c7caf0 # v1.1.1
         with:
-          clang-tidy: 20.1.8
+          clang-tidy: 21.1.0
       
       - name: Setup Python env
         uses: actions/setup-python@42375524e23c412d93fb67b49958b491fce71c38 # v5.4.0
@@ -59,6 +60,13 @@ jobs:
 
       - name: Install Python dependencies
         run: python3 -m pip install -r llvm/utils/git/requirements_linting.txt
+
+      - name: Setup ccache
+        uses: hendrikmuhs/ccache-action@a1209f81afb8c005c13b4296c32e363431bffea5 # v1.2.17
+        with:
+          max-size: 2G
+          key: premerge-clang-tidy
+          variant: sccache
       
       # TODO: create special mapping for 'codegen' targets, for now build predefined set
       # TODO: add entrypoint in 'compute_projects.py' that only adds a project and its direct dependencies

@@ -59,6 +60,13 @@ jobs:

- name: Install Python dependencies
run: python3 -m pip install -r llvm/utils/git/requirements_linting.txt

- name: Setup ccache
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One problem I faced with sccache is that it doesn't store/restore results when I run multiple the job multiple times here.
The warnings are:
Warning: Cache not found for keys: sccache-premerge-clang-tidy-
Warning: Saving cache failed: Error: Path Validation Error: Path(s) specified in the action for caching do(es) not exist, hence no cache is being saved.

I suspect I need to create this key in somewhere or it will start to work after PR is merged into main?

I can see in other workflows similar setup of sccache,

- name: Setup ccache
uses: hendrikmuhs/ccache-action@a1209f81afb8c005c13b4296c32e363431bffea5 # v1.2.17
with:
# A full build of llvm, clang, lld, and lldb takes about 250MB
# of ccache space. There's not much reason to have more than this,
# because we usually won't need to save cache entries from older
# builds. Also, there is an overall 10GB cache limit, and each
# run creates a new cache entry so we want to ensure that we have
# enough cache space for all the tests to run at once and still
# fit under the 10 GB limit.
# Default to 2G to workaround: https://github.com/hendrikmuhs/ccache-action/issues/174
max-size: 2G
key: post-commit-analyzer
variant: sccache

Apart from this problem, shall I create a dedicated key for "clang-tidy" runs or reuse some existing one? In theory this ccache should store a little data because all it only compile codegen targets.

Copy link
Contributor

@boomanaiden154 boomanaiden154 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isolating just the version bump into this change would probably be a good idea. That is straight forward and can just land.

Caching + container changes are going to require more discussion.

@@ -19,8 +19,6 @@ jobs:
defaults:
run:
shell: bash
container:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we dropping this? The default compiler in Github Actions is going to be much slower.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When setup-cpp action is run, it "fails" with such error:

Installing [email protected] via npm...
Error: Failed to install the [email protected] CLI: Error: Command failed with ENOENT: npm install -g [email protected]
spawn npm ENOENT. Ignoring...
✅ clang-tidy was installed successfully:

"fails" is quoted because it clang-tidy still is installed in the path. The only drawback is this annotation in UI:
image

From logs, it seems there are some problems with systemd during installation:

  /usr/lib/tmpfiles.d/systemd-network.conf:10: Failed to resolve user 'systemd-network': No such process
  /usr/lib/tmpfiles.d/systemd-network.conf:11: Failed to resolve user 'systemd-network': No such process
  /usr/lib/tmpfiles.d/systemd-network.conf:12: Failed to resolve user 'systemd-network': No such process
  /usr/lib/tmpfiles.d/systemd-network.conf:13: Failed to resolve user 'systemd-network': No such process
  /usr/lib/tmpfiles.d/systemd.conf:22: Failed to resolve group 'systemd-journal': No such process
  /usr/lib/tmpfiles.d/systemd.conf:23: Failed to resolve group 'systemd-journal': No such process
  /usr/lib/tmpfiles.d/systemd.conf:28: Failed to resolve group 'systemd-journal': No such process
  /usr/lib/tmpfiles.d/systemd.conf:29: Failed to resolve group 'systemd-journal': No such process
  /usr/lib/tmpfiles.d/systemd.conf:30: Failed to resolve group 'systemd-journal': No such process

This leads to this topic, but I can't understand how given answer correlate to us because it said "The host was Ubuntu 20.04 and the image was 24.04.", but we run ubuntu-24 docker image on ubuntu-24 runner.

I found that if I remove this container (which is not used in clang-format) everything installs correctly, and I didn't see significant downgrade of compilation speed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird. We should probably be setting up a container with just clang-tidy and just clang-format since the setup-cpp action takes forever. That's a completely separate action item that needs some refactoring to be done first.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably be setting up a container with just clang-tidy and just clang-format

Very much in favor of it

@@ -59,6 +60,13 @@ jobs:

- name: Install Python dependencies
run: python3 -m pip install -r llvm/utils/git/requirements_linting.txt

- name: Setup ccache
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not going to give you any caching across PRs due to how the caches are namespaced.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's unfortunate. Is there a way to make it work across PRs? Build time of codegen targets is 4-5mins and it is sometimes similar to the whole CI-Linux run which builds whole clang;clang-tools-extra and run tests. I suspect CI checks are fast because they run on llvm-premerge-linux-runners with many cores, but sccache must do a big job too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have to also build against main. Probably not something that we want to do, but we could discuss doing it.

@vbvictor vbvictor force-pushed the improve-clang-tidy-ci branch from 1a060de to 2f3e0b4 Compare September 24, 2025 18:42
@vbvictor vbvictor changed the title [GitHub] Bump clang-tidy version to 21th and add sccache in CI [GitHub] Bump clang-tidy version to 21th in CI Sep 24, 2025
@vbvictor
Copy link
Contributor Author

I removed non-related changes from current PR

Copy link
Contributor

@boomanaiden154 boomanaiden154 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@vbvictor vbvictor merged commit 7813da3 into llvm:main Sep 24, 2025
12 checks passed
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Oct 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants